r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

Haven’t tried understanding the code in actual detail, but I’m wondering why the implications are separated for the const and non-const case. Same feature flag could gate some normal #[unstable] items and also some other #[rustc_const_unstable] items, couldn’t it? If the feature gates can overlap, wouldn’t a unified table of implications make sense, too?

While I’m at it, commenting on the general design… one small downside of the implied_by directionality (as opposed to implies) is that the setting I layed out in the original Zulip thread where a piece of API could be moved to a new feature flag multiple times is no longer so well supported. I do see benefits with this directionality, in particular that the stabilized items will never need to be touched again, but it makes me wonder whether perhaps also allowing a list of multiple features in the implied_by field could be reasonable.

Feel free to ignore this second part of the comment though, as it’s not directly relevant to the PR at hand, I suppose. (And also the use-case should be rather uncommon (and we’re still talking about unstable features anyways), so it’s not a pressing issue.)

Haven’t tried understanding the code in actual detail, but I’m wondering why the implications are separated for the const and non-const case. Same feature flag could gate some normal #[unstable] items and also some other #[rustc_const_unstable] items, couldn’t it? If the feature gates can overlap, wouldn’t a unified table of implications make sense, too?

This makes sense, I've updated it to this.

While I’m at it, commenting on the general design… one small downside of the implied_by directionality (as opposed to implies) is that the setting I layed out in the original Zulip thread where a piece of API could be moved to a new feature flag multiple times is no longer so well supported. I do see benefits with this directionality, in particular that the stabilized items will never need to be touched again, but it makes me wonder whether perhaps also allowing a list of multiple features in the implied_by field could be reasonable.

I think I went with the current directionality for the advantage you've noted. Extending the current approach to accept a list of features makes most sense if we were to do this (though I'm not sure if it's worth it if there isn't a need for that).

Thanks for the quick response.

While I’m at it, more small general thoughts where just asking seems faster than me trying to figure it out from the code of testing: Given that previously, apparently implied_by was allowed but silently ignored inside the rustc_const_unstable attribute… I guess the first though/question is: is there – in general – a check that checks what kind of key-value pairs are allowed in such attributes, and if yes, why was it previously allowing the implied_by on the rustc_const_unstable? Do they kind-of “share the syntax”? My point is that we probably don’t ever want implied_by in a #[stable] or #[rustc_const_stable] attribute (right? or is there any usefulness in something like that?), and helping rustc library authors with a lint or error for this seems like a useful idea.

(hope it's fine to reroll reviewer, was autoassigned)

r? compiler

Would be nice if the stability and const stability code was a little more unified in the future.
Having stricter checking around stability attributes as mentioned by @steffahn would also be useful, but that's also something for a follow-up PR if deemed useful.
The implementation here looks good.
@bors r+ rollup

📌 Commit 9127834 has been approved by Nilstrieb

It is now in the queue for this repository.